Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Desktop entry file for Linux app launchers #295 #3351

Closed
wants to merge 4 commits into from
Closed

Desktop entry file for Linux app launchers #295 #3351

wants to merge 4 commits into from

Conversation

yan12125
Copy link
Contributor

See: https://specifications.freedesktop.org/desktop-entry-spec/desktop-entry-spec-latest.html

  • Address review comments

Seems #296 is stale, so I created a replacement with fixes. Differences from the previous pull request are in the second commit.

rom1v pushed a commit that referenced this pull request Jun 27, 2022
rom1v pushed a commit that referenced this pull request Jun 27, 2022
Make Exec= compatible with $PATH configured in .bashrc/ or .zshrc/…

PR #3351 <#3351>
Refs #296 <#296 (comment)>

Signed-off-by: Romain Vimont <[email protected]>
@rom1v
Copy link
Collaborator

rom1v commented Jun 27, 2022

Hi,

Thank you 👍

I added a first commit to replace the hardcoded share/ in meson.build by the datadir variable (on dev branch). Then I rebased/re-split the commits.

Then I added a scrcpy-console.desktop to provide a launcher with a terminal (like scrcpy-console.bat).

The resulting branch is here: pr3351

Please review and test 😉

@yan12125
Copy link
Contributor Author

Awesome improvements! Just one minor suggestion -

For some users, adb is not in default $PATH but the one configured in .bashrc/ or .zshrc/...

"or" indeed makes it clearer, but a slash after .bashrc is somewhat unusual. Probably better with

For some users, adb is not in default $PATH but the one configured in .bashrc, .zshrc, or other shell startup scripts.

(Term "shell startup scripts" is just the first one out of my head. Different people use different terms for those files :D)

scrcpy.desktop works as usual. For scrcpy-console.desktop, the "complex" Exec= line hits some other software. I'm using QTerminal on LXQt, where lxqt/qterminal#961 is needed to correctly parse that line. That's a QTerminal bug, so keeping the current scrcpy-console.desktop is fine.

rom1v added a commit that referenced this pull request Jun 27, 2022
Meson defines a variable for the data directory.

PR #3351 <#3351>
rom1v pushed a commit that referenced this pull request Jun 27, 2022
rom1v pushed a commit that referenced this pull request Jun 27, 2022
Make Exec= compatible with $PATH configured in .bashrc/ or .zshrc/…

PR #3351 <#3351>
Refs #296 <#296 (comment)>

Signed-off-by: Romain Vimont <[email protected]>
rom1v added a commit that referenced this pull request Jun 27, 2022
Add a launcher which opens a terminal, and keep it open in case of
errors (so that the user has time to read error messages).

The behavior is the same as scrcpy-console.bat on Windows.

PR #3351 <#3351>
rom1v added a commit that referenced this pull request Jun 27, 2022
Add a launcher which opens a terminal, and keep it open in case of
errors (so that the user has time to read error messages).

The behavior is the same as scrcpy-console.bat on Windows.

PR #3351 <#3351>
@rom1v
Copy link
Collaborator

rom1v commented Jun 27, 2022

(Term "shell startup scripts" is just the first one out of my head. Different people use different terms for those files :D)

In man bash:

       ~/.bashrc
              The individual per-interactive-shell startup file

So "shell startup script" looks good to me.

For some users, adb is not in default $PATH but the one configured in .bashrc, .zshrc, or other shell startup scripts.

I suggest:

+# For some users, the PATH or ADB environment variables are set from the shell
+# startup file, like .bashrc or .zshrc… Run an interactive shell to get
+# environment correctly initialized.

What do you think? See pr3351.2.

For scrcpy-console.desktop, the "complex" Exec= line hits some other software. I'm using QTerminal on LXQt, where lxqt/qterminal#961 is needed to correctly parse that line.

😞

That's a QTerminal bug, so keeping the current scrcpy-console.desktop is fine.

OK

@yan12125
Copy link
Contributor Author

Looks great, thank you very much for the efforts!

@rom1v
Copy link
Collaborator

rom1v commented Jun 28, 2022

@asnelling as the author of #296, any remark?

Comment on lines 5 to 7
# For some users, `adb` is not in default $PATH but the one configured in .bashrc/.zshrc/...
# Run an interactive shell to get the same path used in terminals.
Exec=/bin/sh -c '"$SHELL" -i -c scrcpy'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For system-wide installations, Exec should use the system-wide adb installed at, e.g.: /usr/bin/adb and leave customization concerns up to the user in e.g.: ~/.local/applications/scrcpy.desktop
👍 good to merge

@asnelling
Copy link
Contributor

IMO, Exec= should let the system resolve scrcpy and not invoke a shell. Users customizations belong in ~/.local/share/applications/scrcpy.desktop . It seems the convention (at least in Ubuntu) is Exec=scrcpy or Exec=/usr/bin/scrcpy, which you can see with:

$ grep '^Exec=' /usr/share/applications/*.desktop

More importantly, merge it 👍

@yan12125
Copy link
Contributor Author

For system-wide installations, Exec should use the system-wide adb installed at, e.g.: /usr/bin/adb

It's up to whether rom1v wants to support $PATH or $ADB configured in .bashrc/.zshrc/... or not. I'm fine with either.

It seems the convention (at least in Ubuntu) is Exec=scrcpy or Exec=/usr/bin/scrcpy

It's also possible to extract those "complex" Exec= lines to standalone scripts like /usr/bin/scrcpy-console.sh and /usr/bin/scrcpy-noconsole.sh, and use them in Exec=.

@rom1v
Copy link
Collaborator

rom1v commented Jun 29, 2022

IMO, Exec= should let the system resolve scrcpy and not invoke a shell.

So, for the non-console version:

Exec=/usr/bin/scrcpy

For the console version:

Exec=/usr/bin/scrcpy || bash -i -c 'read -p "Press any key to quit..."'

?

Where should the user define ADB or PATH so that it is used for the launcher in that case?

@yangfl any opinion about the content of Exec=?

@yangfl
Copy link
Contributor

yangfl commented Jun 30, 2022

I think you can't use the shell syntax in Exec=.

"$SHELL" -i -c scrcpy

Strange, but acceptable as long as the software author satisfies with it.

@yan12125
Copy link
Contributor Author

yan12125 commented Jul 1, 2022

Exec=/usr/bin/scrcpy || bash -i -c 'read -p "Press any key to quit..."'

I think you can't use the shell syntax in Exec=.

Yeah, || is passed as an argument and failed. Here is a working example with waiting and no interactive shell:

Exec=/bin/bash -c 'scrcpy || read -p "Press any key to quit..."'

"$SHELL" -i -c scrcpy
Strange, but acceptable as long as the software author satisfies with it.

I checked and found that qterminal and similar programs expand environment variables. I'm not sure if other terminal emulators do that or not.

Where should the user define ADB or PATH so that it is used for the launcher in that case?

That depends on how an X11 or Wayland session is started. Apparently there is no universal way - see https://wiki.archlinux.org/title/Environment_variables#Graphical_environment

@rom1v
Copy link
Collaborator

rom1v commented Jul 1, 2022

Yeah, || is passed as an argument and failed. Here is a working example with waiting and no interactive shell:

Exec=/bin/bash -c 'scrcpy || read -p "Press any key to quit..."'

OK. But then the scrcpy-console version will load environment variables from .bashrc (whatever the default shell), while the non-console version will not.

"$SHELL" -i -c scrcpy

Strange, but acceptable as long as the software author satisfies with it.

OK, so maybe I will keep pr3351.2.

Thank you.

@yan12125
Copy link
Contributor Author

yan12125 commented Jul 1, 2022

But then the scrcpy-console version will load environment variables from .bashrc (whatever the default shell), while the non-console version will not.

Yeah that's an issue. How about /bin/bash --norc --noprofile ...?

rom1v and others added 4 commits July 9, 2022 02:56
Meson defines a variable for the data directory.

PR #3351 <#3351>
Refs <https://specifications.freedesktop.org/desktop-entry-spec/desktop-entry-spec-latest.html>

PR #3351 <#3351>
Replaces PR #296 <#296>
Fixes #295 <#295>
Fixes #748 <#748>
Fixes #1636 <#1636>

Co-authored-by: Addison Snelling <[email protected]>
Co-authored-by: Chih-Hsuan Yen <[email protected]>
Co-authored-by: Romain Vimont <[email protected]>
Signed-off-by: Romain Vimont <[email protected]>
Make Exec= compatible with $PATH configured in .bashrc/ or .zshrc/…

PR #3351 <#3351>
Refs #296 <#296 (comment)>

Co-authored-by: Romain Vimont <[email protected]>
Signed-off-by: Romain Vimont <[email protected]>
Add a launcher which opens a terminal, and keep it open in case of
errors (so that the user has time to read error messages).

The behavior is the same as scrcpy-console.bat on Windows.

PR #3351 <#3351>
@yan12125
Copy link
Contributor Author

yan12125 commented Jul 8, 2022

How about /bin/bash --norc --noprofile ...?

Assuming no objection for this approach, I imported pr3351.2 branch, added proposed bash arguments and force-updated this PR.

rom1v added a commit that referenced this pull request Aug 29, 2022
Meson defines a variable for the data directory.

PR #3351 <#3351>
rom1v pushed a commit that referenced this pull request Aug 29, 2022
rom1v pushed a commit that referenced this pull request Aug 29, 2022
Make Exec= compatible with $PATH configured in .bashrc or .zshrc…

PR #3351 <#3351>
Refs #296 <#296 (comment)>

Signed-off-by: Romain Vimont <[email protected]>
rom1v added a commit that referenced this pull request Aug 29, 2022
Add a launcher which opens a terminal, and keep it open in case of
errors (so that the user has time to read error messages).

The behavior is the same as scrcpy-console.bat on Windows.

PR #3351 <#3351>
rom1v added a commit that referenced this pull request Sep 9, 2022
Meson defines a variable for the data directory.

PR #3351 <#3351>
rom1v pushed a commit that referenced this pull request Sep 9, 2022
rom1v pushed a commit that referenced this pull request Sep 9, 2022
Make Exec= compatible with $PATH configured in .bashrc or .zshrc…

PR #3351 <#3351>
Refs #296 <#296 (comment)>

Signed-off-by: Romain Vimont <[email protected]>
rom1v added a commit that referenced this pull request Sep 9, 2022
Add a launcher which opens a terminal, and keep it open in case of
errors (so that the user has time to read error messages).

The behavior is the same as scrcpy-console.bat on Windows.

PR #3351 <#3351>
@rom1v
Copy link
Collaborator

rom1v commented Sep 9, 2022

Merged into dev.

@yan12125
Copy link
Contributor Author

Thanks!

@kidonng kidonng mentioned this pull request Nov 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants